ostree commit: Fix combining trees with multiple --tree=ref arguments

You'd expect

    ostree commit --tree=ref=A --tree=ref=B

to produce a commit with the union of the trees given.  Instead you'd get
a commit with the contents of just the latter commit.  This was due to an
optimisation where we'd skip filling out the `files` and `subdirs`
members of the mtree, just filling in the metadata instead.  This backfires
becuase this same code relies on checking the `files` and `subdirs` members
itself to work out whether the mtree is empty.

This commit removes the optimisation, fixing the bug.  Maybe there's a way
to keep the optimisation and still fix the bug but it's not obvious to
me.

Closes: #581
Approved by: cgwalters

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index 6539c26b30b359bb925df55de1744475efb5c3f4..84d1537417ce13dc743ba690e00d72dc9a7ef753 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -2550,16 +2550,6 @@ write_directory_to_mtree_internal (OstreeRepo                  *self,
 
       ostree_mutable_tree_set_metadata_checksum (mtree, ostree_repo_file_tree_get_metadata_checksum (repo_dir));
 
-      /* If the mtree was empty beforehand, the checksums on the mtree can simply
-       * become the checksums on the tree in the repo. Super simple. */
-      if (g_hash_table_size (ostree_mutable_tree_get_files (mtree)) == 0 &&
-          g_hash_table_size (ostree_mutable_tree_get_subdirs (mtree)) == 0)
-        {
-          ostree_mutable_tree_set_contents_checksum (mtree, ostree_repo_file_tree_get_contents_checksum (repo_dir));
-          ret = TRUE;
-          goto out;
-        }
-
       filter_result = OSTREE_REPO_COMMIT_FILTER_ALLOW;
     }
   else

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index ad70522acac7f0034a97f40fd0b6b5f5052c39b0..bb3875018436f7668e98b3712eee05fd8d28bcf3 100755 (executable)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -19,7 +19,7 @@
 
 set -euo pipefail
 
-echo "1..59"
+echo "1..60"
 
 $OSTREE checkout test2 checkout-test2
 echo "ok checkout"
@@ -194,6 +194,20 @@ cd ${test_tmpdir}/checkout-test2-4
 $OSTREE commit -b test2 -s "no xattrs" --no-xattrs
 echo "ok commit with no xattrs"
 
+mkdir tree-A tree-B
+touch tree-A/file-a tree-B/file-b
+
+$OSTREE commit -b test3-1 -s "Initial tree" --tree=dir=tree-A
+$OSTREE commit -b test3-2 -s "Replacement tree" --tree=dir=tree-B
+$OSTREE commit -b test3-combined -s "combined tree" --tree=ref=test3-1 --tree=ref=test3-2
+
+$OSTREE checkout test3-combined checkout-test3-combined
+
+assert_has_file checkout-test3-combined/file-a
+assert_has_file checkout-test3-combined/file-b
+
+echo "ok commit combined ref trees"
+
 # NB: The + is optional, but we need to make sure we support it
 cd ${test_tmpdir}
 cat > test-statoverride.txt <<EOF